-
Notifications
You must be signed in to change notification settings - Fork 1k
Damienbod/2-2-TokenCache small improvements to readme and small code clean up #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Damienbod/2-2-TokenCache small improvements to readme and small code clean up #411
Conversation
@jmprieur Is the Greetings Damien |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @damienbod
Do you have a strong feeling about the indentation in the startup.cs. The builder do a find of funnel.
@@ -26,7 +26,7 @@ It leverages the ASP.NET Core OpenID Connect middleware and Microsoft Authentica | |||
|
|||
To run this sample, you'll need: | |||
|
|||
- [Visual Studio 2017](https://aka.ms/vsdownload) or just the [.NET Core SDK](https://www.microsoft.com/net/learn/get-started) | |||
- [Visual Studio 2019](https://aka.ms/vsdownload) or just the [.NET Core SDK](https://www.microsoft.com/net/learn/get-started) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we want to keep giving version :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no I don't think this makes sense, I'll remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
.EnableTokenAcquisitionToCallDownstreamApi(initialScopes) | ||
.AddMicrosoftGraph(Configuration.GetSection("DownstreamApi")) | ||
.AddDistributedTokenCaches(); | ||
.AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the indentation is really based on the builders ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @damienbod for reacting so quickly to the feedback!
Purpose
2-2-TokenCache small improvements to readme and small code clean up
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?